Fixes for BuildAsStandalone in tests #84374
Conversation
…(Directory.Build.props sets as default)
…lation should work
|
Tagging subscribers to this area: @hoyosjs Issue DetailsThis should fix test builds with
Admittedly, I still do not understand fully how RequiresProcessIsolation and BuildAsStandalone work. Both effectively do the same thing (use the .cmd route for a test), but they are implemented differently. RequiresProcessIsolation is valid in test project files and isn't used elsewhere. BuildAsStandalone is set in some test infrastructure projects but does not work in specific test project files; it does work in an environment variable for tests. A lot of the complexity is that we don't want boilerplate in individual test project files, but Directory.Build.props runs very early (before per-project settings) and Directory.Build.targets runs very late (after SDK .targets files). Therefore we don't have a good place for default logic that consumes test-specific values. I'm definitely open to ideas. One would be to add an Import at the end of every test project file. At least it would only be one line (and could replace OutputType=Library in the small set that does that), and perhaps it could simplify some of the logic in the D.B.props and .targets files but I'm not sure.
|
|
PTAL @trylek @jkoritzinsky - especially if you have comments about D.B.props/targets and how to structure the build files cc @BruceForstall @dotnet/jit-contrib |
markples
added
test-bug
There was a problem hiding this comment.
Looks great to me, thanks Mark for all these cleanups and fixes! I'll spend some more time thinking about your suggestions for further cleanups regarding BuildAsStandalone and related, I'm certainly all for simplifying the test build logic that is already overcomplicated.
|
/azp run runtime-coreclr outerloop, runtime-extra-platforms |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr outerloop, runtime-extra-platforms |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
The enabledisable test failure on Android will be fixed by #84409 |
|
outerloop green, extra-platforms looks consistent with other jobs (this change is not as disruptive at things like test merging, so I wasn't expecting a difference across platforms) |
ghost
locked as resolved and limited conversation to collaborators
This should fix test builds with
set BuildAsStandalone=true. Viewing the individual commits will help because the first one touches a lot of files.<BuildAsStandalone>false</BuildAsStandalone>from various HardwareIntrinsics tests. This is already the default via Directory.Build.props but interferes withset BuildAsStandalone=true. (nit - also remove an emptyPropertyGroupfrom GitHub_43569.csproj)Mainmethods. We might be able to pursue a strategy where the test author provides aMain(args)method for local custom execution. The test would either need a separate parameterless [Fact] method or a [Theory] plus data forMainfor merged group to execute it. However, a RequiresProcessIsolation/BuildAsStandalone build would ignore that metadata, so I'd like to avoid that complexity. The Main methods here set a bool or parse an int, which can be added locally to any test as needed.Admittedly, I still do not understand fully how RequiresProcessIsolation and BuildAsStandalone work. Both effectively do the same thing (use the .cmd route for a test), but they are implemented differently. RequiresProcessIsolation is valid in test project files and isn't used elsewhere. BuildAsStandalone is set in some test infrastructure projects but does not work in specific test project files; it does work in an environment variable for tests. A lot of the complexity is that we don't want boilerplate in individual test project files, but Directory.Build.props runs very early (before per-project settings) and Directory.Build.targets runs very late (after SDK .targets files). Therefore we don't have a good place for default logic that consumes test-specific values.
I'm definitely open to ideas. One would be to add an Import at the end of every test project file. At least it would only be one line (and could replace OutputType=Library in the small set that does that), and perhaps it could simplify some of the logic in the D.B.props and .targets files but I'm not sure.
Fixes #76421
Fixes #81984